Initial Edits for Basics Section #228
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Good changes! A few notes I'd make about conventions
|
|
I just committed a version that made updates based on your comment. I'm not sure if the new commit will appear in this PR request, for me, it is only showing that I made 1 commit today, but in my terminal on VSCode it says I successfully made the 2nd commit. |
|
For stream_nwb.ipynb, good eye on the markdown! I like that you're considering those things because we want the explanations to be clear and accurate. I'd say the past tense markdown text was more correct, since in the table of contents, Exploring an NWB File comes before Streaming an NWB File with fsspec. So these changes are probably unnecessary |
|
Note that there are merge conflicts from these changes. I think this wouldn't happen with a normal code base but it is happening with Jupyter notebook. This is because when the notebook is rerun on your machine, it changes all of the output cells in the notebook. We should probably discuss the best way to navigate this because git doesn't account for this kind of change. For posterity's sake I'll describe this here; For now, the best thing to do to avoid these merge conflicts in the PR is the make sure to |
rcpeene
changed the title
|
Additional note on commit messages. Instead of referring to something like "changes based on PR" it might be better to describe concretely what those changes are (to the extent that is possible in a short message; commit messages don't need to be super descriptive). If you want to reference a PR in a commit, you can just write PR #123 at the end of the commit message and github will tag it. |
|
Also remove hellworld1.ipnyb |
updated table of contents: swapped positions of Streaming NWB and Exploring NWB because they are out of order.
|
I think all of these changes have been made. |
| "cell_type": "code", | ||
| "execution_count": 3, | ||
| "id": "67536d37", | ||
| "metadata": {}, |
There was a problem hiding this comment.
Line #2. dandiset_id = "000021" #the set ID can be found under the title of the dandiset
- comments referring to a line are typically above the line.
- spaces after # signs
- I think most users will probably have a sense for the filepath usage, but maybe a comment above the line that says "# relative filepath" would suffice.
Reply via ReviewNB
| "attachments": {}, | ||
| "cell_type": "markdown", | ||
| "id": "5c8b955c", | ||
| "metadata": {}, |
There was a problem hiding this comment.
This could be reworded/restructured to seem a bit more natural with the rest of the text. Perhaps something like "In the cells below, the dandi api i used to facilitate the download. the method... etc. etc. and get_asset_by_path does blah"
Reply via ReviewNB
| "cell_type": "code", | ||
| "execution_count": 3, | ||
| "id": "67536d37", | ||
| "metadata": {}, |
There was a problem hiding this comment.
Line #3. download_loc = "."
haha my last feedback must have been confusing.
The 'relative filepath' comment would go above the line with the filepath (download_loc...)
The comment about the dandiset id was good info. If you want to include instructions on that it would go well in the markdown, right after where it says "use its ID in dandiset_id."
Reply via ReviewNB
| "attachments": {}, | ||
| "cell_type": "markdown", | ||
| "id": "5c8b955c", | ||
| "metadata": {}, |
There was a problem hiding this comment.
There was a problem hiding this comment.
I believe it already is one paragraph?
| "attachments": {}, | ||
| "cell_type": "markdown", | ||
| "id": "ae798e1c", | ||
| "metadata": {}, |
There was a problem hiding this comment.
| "dandi_filepath = \"sub-699733573/sub-699733573_ses-715093703.nwb\"\n", | ||
| "download_loc = \".\"" | ||
| ] | ||
| }, |
There was a problem hiding this comment.
| @@ -0,0 +1,61 @@ | |||
| { | |||
There was a problem hiding this comment.
For step one, it reads as though you mean "click here to access any dandiset from the allen institute". Maybe say "access an example 2-Photon Dandiset from the allen institute openscope project"
Reply via ReviewNB
| @@ -0,0 +1,61 @@ | |||
| { | |||
There was a problem hiding this comment.
No description provided.